-
Notifications
You must be signed in to change notification settings - Fork 38.6k
Feature/allowed method interceptor #35273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Feature/allowed method interceptor #35273
Conversation
Signed-off-by: SRIRAM9487 <sriram9487tk@gmail.com>
Added comprehensive unit tests covering all branches of MethodInterceptor: - preHandle: allowed and disallowed methods - postHandle: allowed and disallowed methods - afterCompletion: allowed and disallowed methods Signed-off-by: SRIRAM9487 <sriram9487tk@gmail.com>
845a609
to
7161bdc
Compare
@bclozel Thanks for the feedback on #35272. Understood that this feature is not currently planned. I'll consider maintaining it as an external utility or extension module, since I still believe this helps reduce boilerplate and improves clarity in interceptor setups. Appreciate your time and the review! |
We have not declined nor reviewed this yet. We don't need both an issue and a PR so we closed the other one. |
Thanks for the clarification, @bclozel. |
Until then (the review by those grannies) : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from the comments below, generally the request seems reasonable with the following implementation related points:
- The existing
MappedInterceptor
could be enhanced instead to support mapping by HTTP method. - The terminology
allowedMethods
could be justhttpMethods
, orinclude/excludeHttpMethods
for consistency withinclude/excludePatterns
.
<factorypathentry kind="EXTJAR" id="/home/sriram/.gradle/caches/modules-2/files-2.1/org.pcollections/pcollections/4.0.1/59f3bf5fb28c5f5386804dcf129267416b75d7c/pcollections-4.0.1.jar" enabled="true" runInBatchMode="false"/> | ||
<factorypathentry kind="EXTJAR" id="/home/sriram/.gradle/caches/modules-2/files-2.1/com.uber.nullaway/nullaway/0.12.7/1a813b7d156768204b0c8d52ba0632a8db6fbe12/nullaway-0.12.7.jar" enabled="true" runInBatchMode="false"/> | ||
<factorypathentry kind="EXTJAR" id="/home/sriram/.gradle/caches/modules-2/files-2.1/com.google.guava/failureaccess/1.0.3/aeaffd00d57023a2c947393ed251f0354f0985fc/failureaccess-1.0.3.jar" enabled="true" runInBatchMode="false"/> | ||
</factorypath> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These seem to have been committed by mistake. Could you please remove them from the pull request.
/** | ||
* Add patterns for URLs the interceptor should be included in. | ||
* <p>For pattern syntax see {@link PathPattern} when parsed patterns | ||
* <p> | ||
* For pattern syntax see {@link PathPattern} when parsed patterns |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a number of Javadoc style changes, probably applied by the IDE. Could you please undo those? They are not related and in any case inconsistent with the rest of the codebase.
* @since 5.0.3 | ||
*/ | ||
public InterceptorRegistration addPathPatterns(List<String> patterns) { | ||
this.includePatterns = (this.includePatterns != null ? | ||
this.includePatterns : new ArrayList<>(patterns.size())); | ||
this.includePatterns = (this.includePatterns != null ? this.includePatterns : new ArrayList<>(patterns.size())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise there are a number of code style changes. Please, undo those.
Support HTTP method-restricted interceptors in
InterceptorRegistration
This PR implements #35272, adding support for restricting
HandlerInterceptor
registration to specific HTTP methods.Motivation
Spring MVC currently allows interceptors to be applied based on path patterns, but not HTTP methods. When method-based filtering is needed, it must be manually implemented within the interceptor itself:
This approach clutters interceptor logic with method checks and duplicates similar logic across interceptors when needed. Centralizing HTTP method filtering in the registration API provides a cleaner and more declarative configuration style.
Solution
This PR introduces a new method to
InterceptorRegistration
:This allows restricting the interceptor to specific HTTP methods like
GET
,POST
, etc. Under the hood, the framework wraps the givenHandlerInterceptor
in a newMethodInterceptor
that delegates calls only when the request's HTTP method matches one of the allowed methods.The interceptor will now be invoked only for POST and PUT requests matching
/api/**
.Implementation Details
InterceptorRegistration
stores allowed HTTP methods as aSet<String>
.MethodInterceptor
.MethodInterceptor
checks the request's method before delegating to the original interceptor.Tests
A dedicated test class
MethodInterceptorTests
is included to validate:preHandle
,postHandle
,afterCompletion
) are covered.Example
In this example,
AuditInterceptor
will apply only forGET
requests under/admin/**
.Compatibility
MappedInterceptor
,HandlerInterceptor
) without exposing new public types.